Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
It's ready for review. |
brianschubert
left a comment
brianschubert
left a comment
There was a problem hiding this comment.
Did an initial pass, see comments below. Please check the allowlist entries; the few I spot checked seem to be valid errors that should be corrected. Thanks!
| cysqlite.Connection | ||
| cysqlite.Connection.commit |
There was a problem hiding this comment.
Please add comment(s) indicating why the allowlist entries are necessary.
Spot checking the first few errors from this run: https://github.com/python/typeshed/actions/runs/22920350806/job/66516765808, many of these look like valid errors that should be corrected in the stubs.
For example:
error: cysqlite.Connection is marked as @disjoint_base, but also marked as @final; remove @disjoint_base
Stub: in file /home/runner/work/typeshed/typeshed/stubs/cysqlite/cysqlite/__init__.pyi:108
<TypeInfo cysqlite._cysqlite.Connection>
Runtime:
<class 'cysqlite._cysqlite.Connection'>
This is correct - Connection shouldn't be marked as both @final and @disjoint_base.
error: cysqlite.Connection.commit is inconsistent, runtime does not have parameter "begin"
Stub: in file /home/runner/work/typeshed/typeshed/stubs/cysqlite/cysqlite/__init__.pyi:162
def (self: cysqlite._cysqlite.Connection, begin: builtins.bool =) -> Any
Runtime:
def (self)
This appears correct - the runtime method does not have a begin parameter: https://github.com/coleifer/cysqlite/blob/861ea9e7965c4770e29cc50a726bd06164ca47a8/src/cysqlite/_cysqlite.pyx#L1111
There was a problem hiding this comment.
there is multiple different ways commit is used there is a couple commit functions where begin is needed unless it's is in the main Connection class and Cursor class. Those are the only few that do not require a begin. The other in relation to Atomic, such as a Savepoint and Transaction need it to be visable.
| restart: bool | ||
| name: str | Buffer | None | ||
|
|
||
| SHUTDOWN: object |
There was a problem hiding this comment.
Constants can be marked as Final.
(review note: this sentinel isn't exposed to the user, so there's no need to use the usual sentinel patterns)
| check_table_sizes: bool = False, | ||
| dry_run: bool = False, | ||
| ) -> Cursor: ... | ||
| def pragma(self, key: str, value: Any = ..., database: str | None = None, multi: bool = False) -> None: ... |
There was a problem hiding this comment.
| def pragma(self, key: str, value: Any = ..., database: str | None = None, multi: bool = False) -> None: ... | |
| def pragma(self, key: str, value: Any = sentinel, database: str | None = None, multi: bool = False) -> None: ... |
and import from _typeshed import sentinel. See the definition in _typeshed for details
Co-authored-by: Brian Schubert <brianm.schubert@gmail.com>
Co-authored-by: Brian Schubert <brianm.schubert@gmail.com>
Co-authored-by: Brian Schubert <brianm.schubert@gmail.com>
Co-authored-by: Brian Schubert <brianm.schubert@gmail.com>
Co-authored-by: Brian Schubert <brianm.schubert@gmail.com>
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
2 participants
I have created type stubs for cysqlite so that a separate package does not need inventing I have a library that needs these type hints called anyio-cysqlite that uses this package and having type hints that are readily accessible and available for it is extremely helpful. I have already made a pull request to that repo and having had it turned down I decided to put everything into here instead.
SEE:
The alternative and final option is to maintain it myself which I do not see as being a feasible option at the moment.
fixes #15469